Skip to content

fix: scrub the revoked key-slot table from every superblock slot (#103) - #156

Merged
Xof merged 1 commit into
mainfrom
fix/103-key-slot-residue
Aug 5, 2026
Merged

fix: scrub the revoked key-slot table from every superblock slot (#103)#156
Xof merged 1 commit into
mainfrom
fix/103-key-slot-residue

Conversation

@Xof

@Xof Xof commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Closes #103. Stacked on #154.

CRYPTO-1 — a revoked credential could still read the live database

rewrite_crypto_header wrote the new key-slot table into exactly one superblock slot. The other N-1 kept their own state, and that state includes their copy of the table. The table is cleartext at bytes 332..1356 of every superblock page, and the per-DB DEK never changes — so a revoked credential's wrapped DEK stayed sitting in the live file.

The attack needs no tampering, no older file image, and no rollback:

  1. Hold the revoked credential and read access to the current file.
  2. Parse the sibling slot's 128-byte record at offset 332.
  3. Run derive_kek + unwrap_dek with slot.aad().
  4. Recover the DEK that still seals every current page and superblock body.

Chisel::open refusing the old key is not protection when the material to bypass it sits in the same file. This is distinct from the rollback exposure THEORY.md already documents, which requires substituting a wholly older image — here the stale credential lives inside the current one. The residue cleared only when N-1 further commits happened to overwrite every sibling; for an idle database, never.

The fix

overwrite_slot_table patches only the table region of a sibling and re-stamps the checksum, so each sibling keeps its own counter, roots and sealed body and remains a valid shadow-paging fallback. That is safe because the body's AAD (sb_identity_aad) covers magic, format_version, txn_counter and superblock_count — not the crypto header.

The scrubs run after the target-slot write and share its fsync, so a partial write can never cost the revocation itself. Worst case a sibling stays un-scrubbed, which is no worse than the steady state this replaces, and the next key operation clears it.

The test asserts on bytes, not behaviour: it captures the old credential's wrapped_dek before revoking, then requires those 32 bytes to appear in no key slot of no superblock slot afterwards. With the scrub disabled it fails on superblock slot 0, key slot 0.

The generation counter — why it is deliberately not here

The issue (and the plan for this PR) called for binding a per-header generation counter into KeySlot::aad() so a spliced-in older slot fails authentication. On inspection that does not achieve what it was asked to.

A slot spliced in from an older image carries its own generation in its own AAD input. aad() is computed from the slot's stored bytes, so the AAD stays self-consistent and the unwrap still succeeds — the attacker moved the evidence along with the crime. Making a generation actually bite requires storing the authoritative value in the sealed body and cross-checking it after decryption, which is a different (and larger) mechanism, and is squarely the per-page temporal-replay problem tracked as #142.

The all-slot scrub closes the finding as filed, with no on-disk format change and no compatibility break. Happy to do the sealed-body cross-check as follow-up work under #142 if wanted.

Also: the contract rotate_key was missing

"After this returns, old no longer opens the database" was true, and is now more true — but it is still revocation, not cryptographic erasure. The DEK is unchanged, so:

  • anyone who captured the DEK while old was valid keeps reading, including pages written afterwards;
  • a backup taken before the revocation is still fully readable with old.

Neither is a defect; both are properties of not re-keying. An operator reaching for this during an incident needs to know which one they are getting, so rotate_key now says so and remove_key points at it.

The third #103 finding

SWIFT-6 (ChiselKey/Options derive Debug, so a {:?} prints the raw passphrase) cannot be fixed on this lineagechisel-ffi/ does not exist on main, and the Swift binding lives on the orphaned pre-v1.0.0 branch with no common ancestor. Filed as #155 to be applied as part of porting that binding.

Verification

719 tests pass, clippy and fmt clean. The new test verified non-vacuous by disabling the scrub.

CRYPTO-1, the last code finding on #103. (SUPERBLOCK-RECOVERY-3 landed with
#117's CRYPTO-4; SWIFT-6 cannot be fixed on this lineage and is now #155.)

`rewrite_crypto_header` wrote the new key-slot table into exactly ONE
superblock slot. The other N-1 kept their own state, and that state includes
their copy of the table. The table is CLEARTEXT at bytes 332..1356 of every
superblock page, and the per-DB DEK never changes — so a revoked credential's
wrapped DEK stayed sitting in the LIVE file.

The attack needs no tampering, no older file image and no rollback. An
adversary holding the revoked credential and mere READ access parses the
sibling slot's 128-byte record, runs derive_kek + unwrap_dek with slot.aad(),
and recovers the DEK that still seals every current page and superblock body.
`Chisel::open` refusing the old key is not protection when the material to
bypass it is in the same file. This is distinct from the rollback exposure
THEORY.md already documents, which needs a wholly older image; here the stale
credential lives inside the CURRENT one. The residue cleared only when N-1
further commits happened to overwrite every sibling — for an idle database,
never.

`overwrite_slot_table` patches only the table region of a sibling and
re-stamps the checksum, so each sibling keeps its own counter, roots and
sealed body and remains a valid shadow-paging fallback. That is safe because
the body's AAD (`sb_identity_aad`) covers magic, format_version, txn_counter
and superblock_count — not the crypto header. The scrubs run after the target
slot write and share its fsync, so a partial write can never cost the
revocation itself; worst case a sibling stays un-scrubbed, which is no worse
than the steady state this replaces, and the next key operation clears it.

The test asserts on the bytes rather than on behaviour: it captures the old
credential's wrapped_dek before revoking and then requires those 32 bytes to
appear in NO key slot of NO superblock slot afterwards. With the scrub
disabled it fails on superblock slot 0, key slot 0.

`rotate_key` also gains the section its contract was missing. "After this
returns, `old` no longer opens the database" was true and is now more true,
but it is still revocation rather than cryptographic erasure: the DEK is
unchanged, so anyone who captured it while `old` was valid keeps reading, and
a backup taken before the revocation is still readable with `old`. Both are
properties of not re-keying, not defects, and an operator reaching for this
during an incident needs to know which one they are getting.

NOT done here, deliberately: binding a per-header generation counter into
`KeySlot::aad()`. On inspection that does not achieve what it was asked to —
a slot spliced in from an older image carries its OWN generation in its own
AAD input, so the AAD stays self-consistent and the unwrap still succeeds.
Making it bite requires storing the authoritative generation in the SEALED
body and cross-checking after decryption, which is the per-page temporal
replay problem (#142) rather than this one. The all-slot scrub above closes
the finding as filed, with no on-disk format change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Encryption and superblock: authentication scope and key-slot handling

1 participant